Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1876: Remove the redundant code for fmt.Print #1878

Merged
merged 2 commits into from
Oct 24, 2024

Conversation

MohitKambli
Copy link
Contributor

Hello,
Hope you're doing well
As per the request mentioned in issue #1876, I have simple removed the redundant log statement that has been mentioned
If at all I missed something or implemented code changes incorrectly, then do let me know
Sincere apologies for any inconvenience or misunderstanding

Thanks and Regards,
Mohit Kambli

@seokho-son
Copy link
Member

Hi @MohitKambli

Thanks for the contribution!
Please remove fmt.Println("UpdateConfig(); Key: " + keyValue.Key + "\nValue: " + keyValue.Value) as well. We don't need it anymore.

@MohitKambli
Copy link
Contributor Author

Hello @seokho-son,
I have made the necessary changes as per the requirement
Kindly have a look at it
If at all there's anything else needed from my end, then do let me know

Thanks and Regards,
Mohit Kambli

@MohitKambli
Copy link
Contributor Author

Hello @seokho-son,
Will it be fine for you if I raise a new PR altogether for this issue?
Sincere apologies for the inconvenience occurred

Thanks and Regards,
Mohit Kambli

@seokho-son
Copy link
Member

Hi @MohitKambli
I guess you've been worried about the build failure in the Checks.

Run make
cd src/ && make
make[1]: Entering directory '/home/runner/work/cb-tumblebug/cb-tumblebug/src'
go build -v -o cb-tumblebug
github.com/cloud-barista/cb-tumblebug/src/api/rest/server/model
github.com/cloud-barista/cb-tumblebug/src/core/model
github.com/cloud-barista/cb-tumblebug/src/api/rest/docs
github.com/cloud-barista/cb-tumblebug/src/core/common/label
github.com/cloud-barista/cb-tumblebug/src/core/common
# github.com/cloud-barista/cb-tumblebug/src/core/common
Error: core/common/config.go:1[5](https://github.com/cloud-barista/cb-tumblebug/actions/runs/11481873195/job/31991517015?pr=1878#step:4:6)0:2: declared and not used: keyValue
make[1]: *** [Makefile:2: default] Error 1
make: *** [Makefile:2: default] Error 2
make[1]: Leaving directory '/home/runner/work/cb-tumblebug/cb-tumblebug/src'
Error: Process completed with exit code 2.

According to the error messages,
We need to remove the following code as well. :)

keyValue, _ := kvstore.GetKv(key)

@@ -149,10 +149,6 @@ func UpdateConfig(u *model.ConfigReq) (model.ConfigInfo, error) {
}
keyValue, _ := kvstore.GetKv(key)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
keyValue, _ := kvstore.GetKv(key)

@seokho-son
Copy link
Member

You don't have to reopen this PR, we can handle the issue in this PR. :)

@seokho-son
Copy link
Member

@MohitKambli
FYI, I applied my suggestion :) So, the check for build test passed.

Copy link
Member

@seokho-son seokho-son left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@seokho-son
Copy link
Member

/approve

@github-actions github-actions bot added the approved This PR is approved and will be merged soon. label Oct 24, 2024
@cb-github-robot cb-github-robot merged commit c80f484 into cloud-barista:main Oct 24, 2024
4 of 5 checks passed
@seokho-son
Copy link
Member

Thanks for the contribution!
Please take a look on this project.
Hope you can find other points to contribute this project!

@MohitKambli
Copy link
Contributor Author

Hello @seokho-son,
Yes, I was quite worried about the failure mentioned in the checks
Thank you for informing me about what exactly I missed
Also, it is really nice of you to explain me what exactly went wrong and how you fixed it
Sincere apologies for my mistakes
It was really kind of you for understanding

Thanks and Regards,
Mohit Kambli

@seokho-son
Copy link
Member

@all-contributors please add @MohitKambli for Code

Copy link
Contributor

@seokho-son

I've put up a pull request to add @MohitKambli! 🎉

@seokho-son
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved This PR is approved and will be merged soon.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants